Update restaurant finder sample #570
Conversation
- Add a base InferenceStrategy class - Add PackSpecsBuildHook to copy JSON schemas into the package during build time. - Update pyproject.toml to include assets and configure the build hook. - Implement A2uiSchemaManager for robust schema loading, pruning, and prompt generation.
Tested: - [x] The contact_lookup client successfully connected to the contact_lookup agent and rendered the response correctly.
Introduces a `schema_modifiers` parameter to A2uiSchemaManager, allowing custom callable hooks to transform schemas after loading. This enables flexible schema customization, such as relaxing strict validation constraints during testing.
It updates the sample to use the A2uiSchemaManager from the a2ui-agent python SDK. Tested: - [x] The `contact` angular client successfully connected to the `contact_multiple_surfaces` agent and rendered the response correctly.
It updates the sample to use the A2uiSchemaManager from the a2ui-agent python SDK. Tested: - [x] The `restaurant` angular client successfully connected to the `restaurant_finder` agent and rendered the response correctly.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new a2ui-agent Python SDK to centralize schema management and updates the existing samples to use it. This is a great architectural improvement that removes hardcoded schemas from the samples and makes them more maintainable. The new A2uiSchemaManager provides a robust way to load, bundle, and validate schemas, with fallbacks for different execution environments. My review focuses on improving the robustness of the new SDK, particularly around exception handling and ensuring functions are side-effect-free.
| except Exception as e: | ||
| raise IOError( | ||
| f"Could not load package resource {filename} in {self.package_path}: {e}" | ||
| ) from e |
There was a problem hiding this comment.
The use of a broad except Exception as e: can catch and hide unexpected errors, making debugging more difficult. It's better to catch more specific exceptions that you anticipate, such as ModuleNotFoundError, FileNotFoundError, or json.JSONDecodeError.
| except Exception as e: | |
| raise IOError( | |
| f"Could not load package resource {filename} in {self.package_path}: {e}" | |
| ) from e | |
| except (ModuleNotFoundError, FileNotFoundError, json.JSONDecodeError) as e: | |
| raise IOError( | |
| f"Could not load package resource {filename} in {self.package_path}: {e}" | |
| ) from e |
| except Exception as e: | ||
| logging.debug("Could not load schema '%s' from package: %s", filename, e) |
There was a problem hiding this comment.
Catching a broad Exception can mask unexpected errors and make debugging difficult. It's better to catch the specific IOError that the loader is expected to raise on failure. This makes the code more robust and its behavior more predictable. This feedback applies to the other broad exception handlers in this file as well.
| except Exception as e: | |
| logging.debug("Could not load schema '%s' from package: %s", filename, e) | |
| except IOError as e: | |
| logging.debug("Could not load schema '%s' from package: %s", filename, e) |
| def remove_strict_validation(schema): | ||
| if isinstance(schema, dict): | ||
| if 'additionalProperties' in schema and schema['additionalProperties'] is False: | ||
| del schema['additionalProperties'] | ||
| for value in schema.values(): | ||
| remove_strict_validation(value) | ||
| elif isinstance(schema, list): | ||
| for item in schema: | ||
| remove_strict_validation(item) | ||
| return schema |
There was a problem hiding this comment.
This function modifies the input schema in-place, which can lead to unexpected side effects for callers who might not expect their original schema object to be altered. It's generally safer for utility functions like this to return a modified copy instead of mutating the input directly. A functional approach that builds and returns a new, modified structure would be more robust.
| def remove_strict_validation(schema): | |
| if isinstance(schema, dict): | |
| if 'additionalProperties' in schema and schema['additionalProperties'] is False: | |
| del schema['additionalProperties'] | |
| for value in schema.values(): | |
| remove_strict_validation(value) | |
| elif isinstance(schema, list): | |
| for item in schema: | |
| remove_strict_validation(item) | |
| return schema | |
| def remove_strict_validation(schema): | |
| if isinstance(schema, dict): | |
| new_schema = {k: remove_strict_validation(v) for k, v in schema.items()} | |
| if 'additionalProperties' in new_schema and new_schema.get('additionalProperties') is False: | |
| del new_schema['additionalProperties'] | |
| return new_schema | |
| elif isinstance(schema, list): | |
| return [remove_strict_validation(item) for item in schema] | |
| return schema |
| # TODO: Implementation logic for Template Manager | ||
| return "" |
There was a problem hiding this comment.
The method generate_system_prompt currently returns an empty string as a placeholder. This could lead to silent failures or unexpected behavior downstream if the caller expects a non-empty prompt. It would be better to make it explicit that this is not implemented yet by raising a NotImplementedError.
| # TODO: Implementation logic for Template Manager | |
| return "" | |
| # TODO: Implementation logic for Template Manager | |
| raise NotImplementedError("This method is not yet implemented.") |
1 participant
Description
It updates the sample to use the A2uiSchemaManager from the a2ui-agent python SDK.
Tested:
restaurantangular client successfully connected to therestaurant_finderagent and rendered the response correctly.Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.